Skip to content

fix(tui): Guard against degenerate scroll region when viewport fills screen - #358

Merged
theahura merged 1 commit into
mainfrom
auto/glad-orb-20260225-222054
Feb 25, 2026
Merged

fix(tui): Guard against degenerate scroll region when viewport fills screen#358
theahura merged 1 commit into
mainfrom
auto/glad-orb-20260225-222054

Conversation

@theahura

Copy link
Copy Markdown
Contributor

Summary

🤖 Generated with Nori

  • Fix frozen rows / scrollback corruption caused by insert_history_lines emitting a degenerate DECSTBM scroll region (\x1b[1;0r) when the viewport fills the entire terminal screen (area.top() == 0)
  • Add early return guard when there's no room above the viewport for history lines
  • Add two tests: one proving the bug (full-screen viewport corruption) and one regression test (history insertion with room above viewport)

Root Cause

SetScrollRegion(1..0) emits \x1b[1;0r. In DECSTBM, Pb=0 means "bottom of screen", so the scroll region covers the entire terminal instead of being empty. Subsequent \r\n + content writes scroll and overwrite viewport rows. Ratatui's diff-based renderer doesn't know the screen was corrupted, producing frozen/stale rows.

Test Plan

  • full_screen_viewport_does_not_corrupt_display — confirms viewport content is unchanged when insert_history_lines is called with a full-screen viewport
  • history_lines_inserted_above_viewport_with_room — confirms normal history insertion still works correctly
  • All 920 existing nori-tui lib tests pass
  • E2E tests pass (tui-pty-e2e)
  • Manual testing in tmux to verify the frozen rows issue no longer occurs

Share Nori with your team: https://www.npmjs.com/package/nori-ai

…screen

When the viewport occupies the entire terminal (area.top() == 0),
insert_history_lines would emit \x1b[1;0r (DECSTBM with Pb=0), which
terminals interpret as "full screen" rather than "empty region". This
caused history content to scroll through and overwrite viewport rows,
producing frozen/stale rows that ratatui's diff renderer could not detect.

Add an early return when area.top() == 0 since there is no room above
the viewport for history lines.
🤖 Generated with [Nori](https://usenori.ai)

Co-Authored-By: Nori <contact@tilework.tech>
@theahura
theahura merged commit 9b56139 into main Feb 25, 2026
3 checks passed
@theahura
theahura deleted the auto/glad-orb-20260225-222054 branch February 25, 2026 23:44
nori-sessions Bot pushed a commit that referenced this pull request Jul 25, 2026
…iewport

insert_history_lines emitted ESC[1;1r when area.top() == 1 — an invalid
DECSTBM region (bottom margin must exceed top) that terminals ignore,
falling back to a full-screen scroll region. Every inserted history line
then scrolled the live viewport itself, and ratatui's diff buffer never
repaired the damage because it believed the viewport was intact. Large
completed-turn outputs made this state both more reachable (tall
viewports) and more destructive (one full-screen scroll per line).

Extend the existing top()==0 guard (#358) to top() <= 1: warn, keep the
call cursor-position-neutral, persist any viewport move made by the
scroll-down branch, and return false so the caller retains pending lines
for retry. The deferral is self-healing: the next insert scrolls the
viewport down further, opening a valid region.

Adds four VT100-backed regression tests: static top()==1 corruption and
return contract, the scroll-down-into-y==1 path, and retry convergence.
🤖 Generated with [Nori](https://noriagentic.com)

Co-Authored-By: Nori <contact@tilework.tech>
CSRessel pushed a commit that referenced this pull request Jul 25, 2026
…iewport (#519)

## Summary
🤖 Generated with [Nori](https://noriagentic.com/)

- `insert_history_lines` emitted an invalid `ESC[1;1r` DECSTBM when
exactly one row remained above the inline viewport (`area.top() == 1`).
Terminals ignore invalid DECSTBM and fall back to a **full-screen**
scroll region, so each inserted history line scrolled the live viewport
itself — persistent display corruption, since ratatui's diff buffer
believed the viewport was intact and never repainted it. Large
completed-turn outputs make this state both more reachable (tall
viewports) and more destructive (one full-screen scroll per line). This
is the unguarded sibling of the `top()==0` fix from #358.
- The guard now covers `area.top() <= 1`: warn, stay
cursor-position-neutral, persist any viewport move made by the preceding
scroll-down branch, and return `false` so `Tui::draw` retains pending
lines. The deferral is self-healing — the next insert scrolls the
viewport down further and succeeds (covered by a retry-convergence
test).
- Four VT100-backed regression tests (the vt100 parser implements the
same degenerate-DECSTBM full-screen fallback as real terminals, verified
against its source) plus `nori-rs/tui/docs.md` updates.

## Follow-ups found during investigation (not in this diff)
- `tui.rs` `scroll_region_up(0..area.top(), …)` on viewport expansion
emits the same invalid `ESC[1;1r` when `area.top() == 1`; masked by the
subsequent `terminal.clear()`, but can push stale viewport rows into
scrollback.
- The `MAX_PENDING_LINES` (1000) trim in `Tui::draw` runs on every draw
before insertion is attempted, so a single >1000-line batch loses its
oldest lines even when insertion succeeds — contrary to its comment
("while blocked").
- Architectural: VS Code terminal, Windows Terminal, tmux, and Zellij
discard region-scrolled lines instead of moving them to scrollback (same
open upstream family: openai/codex #27644, #24849, #15380); upstream
mitigates via resize-triggered transcript reflow (#18575), which this
fork predates.

## Test Plan
- [x] TDD: three repro tests written first and confirmed RED against the
unfixed code (viewport rows visibly overwritten by inserted lines),
GREEN after the guard
- [x] `cargo test -p nori-tui` — 1349 passed, 0 failed; no pending insta
snapshots
- [x] `just fmt` / `just fix -p nori-tui` clean
- [x] `cargo build --bin nori && cargo test -p tui-pty-e2e` — all
runnable suites pass (requires `mock_acp_agent` binary built)
- [x] Closed the loop: drove `./target/debug/nori --agent elizacp` in an
isolated tmux session — full turn completes, reply inserted into history
above the viewport, no corruption

Share Nori with your team: https://www.npmjs.com/package/nori-skillsets

Co-authored-by: Nori <contact@tilework.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant